Benchmark improvements #2415

ZzEeKkAa · 2024-10-02T19:08:15Z

I've picked up some useful changes from #1905 and pushed them here. Also organized python library build.

So basically it is a clean up with some features added and get it ready for the 2025 release

List of changes:

Add benchmark project artifacts to gitignore
Compile python library using dedicated cmake api: https://cmake.org/cmake/help/latest/module/FindPython3.html
Suppress old style warning of xetla library
Prevent cmake build on clean up commands (before that it was unconditional
If there is no ipex, library will be compiled in no ipex mode with user warning
More modular setup.py
Verbose output of cmake commands being run
CMakeLists.txt cleanup
Fix shadow import usage of cmake library

Closes #1905

pbchekin · 2024-10-02T23:18:26Z

benchmarks/CMakeLists.txt

 endif()

-find_package(Python3 COMPONENTS Interpreter)
+find_package(Python 3.9 REQUIRED


Why Python 3.9?

This is the minimum version of python that is requested. Do we support triton from 3.9?

Oh I see, the syntax is a little bit confusing. Why we not specifying the upper end, for example 3.9...3.12?

Sure, we can, but do we want to keep it updating every new python release. I guess I'll update it just for find_package(Python3 REQUIRED. If any issue will appear - we can add restrictions later.

ZzEeKkAa · 2024-10-10T15:05:48Z

Pre-commit checks fails here, but not locally. I've added them to ignore list, but they are still appear in the CI. @pbchekin do you know what I'm missing?

pbchekin · 2024-10-10T16:06:12Z

Pre-commit checks fails here, but not locally. I've added them to ignore list, but they are still appear in the CI. @pbchekin do you know what I'm missing?

What is the ignore list? The complain is legit, distutils is deprecated, https://peps.python.org/pep-0632/#migration-advice.

ZzEeKkAa · 2024-10-10T16:24:26Z

Pre-commit checks fails here, but not locally. I've added them to ignore list, but they are still appear in the CI. @pbchekin do you know what I'm missing?

What is the ignore list?

I mean #noqa comments. It feels that different configurations are being picked in the CI and localy. I'm also getting bunch of errors locally, that does not appear here:

pylint for benchmarks....................................................Failed
- hook id: pylint
- exit code: 16

************* Module triton_kernels_benchmark.benchmark_driver
benchmarks/triton_kernels_benchmark/benchmark_driver.py:12:0: C0411: third party import "torch" should be placed before first party imports "triton.backends.compiler.GPUTarget", "triton.backends.driver.DriverBase", "triton.runtime.cache.get_cache_manager", "triton.runtime.build._build"  (wrong-import-order)
benchmarks/triton_kernels_benchmark/benchmark_driver.py:13:0: C0411: third party import "intel_extension_for_pytorch" should be placed before first party imports "triton.backends.compiler.GPUTarget", "triton.backends.driver.DriverBase", "triton.runtime.cache.get_cache_manager", "triton.runtime.build._build"  (wrong-import-order)
************* Module triton_kernels_benchmark.flash_attention_fwd_benchmark
benchmarks/triton_kernels_benchmark/flash_attention_fwd_benchmark.py:6:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
benchmarks/triton_kernels_benchmark/flash_attention_fwd_benchmark.py:7:0: C0411: third party import "xetla_kernel" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.fused_softmax
benchmarks/triton_kernels_benchmark/fused_softmax.py:15:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language", "triton.runtime.driver"  (wrong-import-order)
benchmarks/triton_kernels_benchmark/fused_softmax.py:16:0: C0411: third party import "xetla_kernel" should be placed before first party imports "triton", "triton.language", "triton.runtime.driver"  (wrong-import-order)
************* Module triton_kernels_benchmark.gemm_benchmark
benchmarks/triton_kernels_benchmark/gemm_benchmark.py:15:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
benchmarks/triton_kernels_benchmark/gemm_benchmark.py:16:0: C0411: third party import "xetla_kernel" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.gemm_postop_addmatrix_benchmark
benchmarks/triton_kernels_benchmark/gemm_postop_addmatrix_benchmark.py:13:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.gemm_postop_gelu_benchmark
benchmarks/triton_kernels_benchmark/gemm_postop_gelu_benchmark.py:15:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.gemm_preop_exp_benchmark
benchmarks/triton_kernels_benchmark/gemm_preop_exp_benchmark.py:14:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.gemm_splitk_benchmark
benchmarks/triton_kernels_benchmark/gemm_splitk_benchmark.py:5:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.gemm_streamk_benchmark
benchmarks/triton_kernels_benchmark/gemm_streamk_benchmark.py:12:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)
************* Module triton_kernels_benchmark.prefix_sums
benchmarks/triton_kernels_benchmark/prefix_sums.py:5:0: C0411: third party import "triton_kernels_benchmark" should be placed before first party imports "triton", "triton.language"  (wrong-import-order)

------------------------------------------------------------------
Your code has been rated at 9.90/10 (previous run: 9.90/10, +0.00)

The complain is legit, distutils is deprecated, https://peps.python.org/pep-0632/#migration-advice.

Indeed. I've dig deeper in the setuptools source code - they vendor distutills.

ZzEeKkAa · 2024-10-10T17:06:25Z

Okay, using vendored distutil from setuptools should be done through import distutils. I bet linter is not aware that distutil was monkey patched (at least partially by setuptools) and should not be considered deprecated.

Importing setuptools._distutils results in a well know issue:
pypa/setuptools#3743

anmyachev · 2024-10-10T21:55:12Z

benchmarks/CMakeLists.txt


-find_package(Python3 COMPONENTS Interpreter)
+find_package(Python3 REQUIRED
+  COMPONENTS Development.Module)


What is the reason for this change?

We don't need interpreter for cmake (like calling python scripts), but we need to compile python library.
You can find out more here: https://cmake.org/cmake/help/latest/module/FindPython.html

benchmarks/xetla_kernel/CMakeLists.txt

anmyachev · 2024-10-10T21:57:15Z

benchmarks/cmake/FindXeTLALibrary.cmake

                  XeTLA library from https://github.com/intel/xetla into
                  ${XeTLALibrary_SOURCE_DIR}")
-    file(READ xetla-library.conf XeTLALibrary_TAG)
+    file(READ xetla_kernel/xetla-library.conf XeTLALibrary_TAG)


I've moved library requirements to the top level cmake.

benchmarks/xetla_kernel/flash_attention/fmha_forward_v5.h

anmyachev · 2024-10-10T22:02:10Z

benchmarks/setup.py

+                                                                              os.path.dirname(__file__)))
+

+class build_ext(_build_ext):


The name does not match the class naming style. CamelCase?

It is a class that overrides specific class of setuptools, so it kept that way to be consistent with the library.

benchmarks/setup.py

anmyachev · 2024-10-10T22:07:15Z

So basically it is a clean up with some features added and get it ready for the 2025 release

@ZzEeKkAa could you highlight the new features at the top of this pull request? This will allow us to use them without reading the code :)

benchmarks/setup.py

anmyachev · 2024-10-11T10:03:19Z

benchmarks/setup.py


-from setuptools import setup
+# TODO: update once there is replacement for clean:
+#  https://github.com/pypa/setuptools/discussions/2838


I read the discussion and it seems like it will never happen.

Should we just switch to pip install command instead of python setup.py install here:

intel-xpu-backend-for-triton/.github/workflows/triton-benchmarks.yml

Line 107 in beffcd1

python setup.py install

?

The replacement is described here: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html#summary

Moreover, the build env setup is usually up to the build front-end (pip or pypa/build, for example) and they often just create "throw-away" virtualenvs under /tmp that you wouldn't need to clean up

Given that pip creates a temporary folder and there is no need to clean it up, we can avoid using the deprecated API.

I'm personally using setup.py as main tool. It also makes challenging to clean up when you build without isolation in pip.

In other words, until they come up with a better approach on cleaning, I would prefer to keep this deprecated API.

Ok for now, but most likely from everything I read, they can't provide such an API anymore, but delegate this task to other tools

anmyachev

LGTM! @ZzEeKkAa please fix conflicts.

@pbchekin ?

pbchekin · 2024-10-15T17:39:57Z

LGTM! @ZzEeKkAa please fix conflicts.

@pbchekin ?

LGTM!

ZzEeKkAa requested review from anmyachev and yudongsi October 2, 2024 19:08

ZzEeKkAa self-assigned this Oct 2, 2024

pbchekin reviewed Oct 2, 2024

View reviewed changes

vlad-penkin linked an issue Oct 3, 2024 that may be closed by this pull request

[Benchmarks] XeTLA kernel python library build #2418

Closed

ZzEeKkAa force-pushed the feature/benchmark_improvements branch 3 times, most recently from 2ef5c79 to 8fe5f9f Compare October 3, 2024 17:19

ZzEeKkAa added the enhancement label Oct 3, 2024

ZzEeKkAa force-pushed the feature/benchmark_improvements branch 3 times, most recently from 245cea0 to 810be92 Compare October 10, 2024 14:13

ZzEeKkAa marked this pull request as ready for review October 10, 2024 14:51

ZzEeKkAa changed the title ~~Feature/benchmark improvements~~ Benchmark improvements Oct 10, 2024

ZzEeKkAa force-pushed the feature/benchmark_improvements branch 2 times, most recently from 6b2f1c7 to e4c05a5 Compare October 10, 2024 16:20

ZzEeKkAa force-pushed the feature/benchmark_improvements branch from e4c05a5 to d11b65d Compare October 10, 2024 16:35

ZzEeKkAa force-pushed the feature/benchmark_improvements branch 4 times, most recently from 45cea57 to cc6a5e2 Compare October 10, 2024 17:42

anmyachev reviewed Oct 10, 2024

View reviewed changes

benchmarks/setup.py Outdated Show resolved Hide resolved

anmyachev reviewed Oct 10, 2024

View reviewed changes

benchmarks/setup.py Outdated Show resolved Hide resolved

anmyachev reviewed Oct 11, 2024

View reviewed changes

anmyachev approved these changes Oct 15, 2024

View reviewed changes

pbchekin approved these changes Oct 15, 2024

View reviewed changes

ZzEeKkAa added 9 commits October 16, 2024 18:52

Add benchmark build files to git ignore

8828753

Improve cmake for benchmarks

152a983

Add missing import for LIMITS in benchmarks

9d952c7

Trigger benchamrks' cmake build only when needed and use build_type

38c0e27

Remove redundant cmake version check at benchmarks

b5f28b4

Incapsulate IPEX check in benchamrks

f8645f4

Integrate cmake into cmdclass for benchmarks

7068f36

Add ext_modules for install build

f4953ee

Apply discussion fixes

368ef37

ZzEeKkAa force-pushed the feature/benchmark_improvements branch from f1b25a9 to 368ef37 Compare October 16, 2024 18:53

ZzEeKkAa merged commit f4fdd8f into intel:main Oct 16, 2024
5 checks passed

Benchmark improvements #2415

Benchmark improvements #2415

Uh oh!

Conversation

ZzEeKkAa commented Oct 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZzEeKkAa commented Oct 10, 2024

Uh oh!

pbchekin commented Oct 10, 2024

Uh oh!

ZzEeKkAa commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZzEeKkAa commented Oct 10, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anmyachev commented Oct 10, 2024

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anmyachev left a comment

Choose a reason for hiding this comment

Uh oh!

pbchekin commented Oct 15, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ZzEeKkAa commented Oct 2, 2024 •

edited

Loading

ZzEeKkAa commented Oct 10, 2024 •

edited

Loading